Add more typing to transaction and base client #2238

jameshilliard · 2024-07-18T17:49:03Z

Also fix some issues identified by additional checks.

janiversen

Changes look good, but there are a couple of CI problems.

jameshilliard · 2024-07-18T18:12:44Z

there are a couple of CI problems

Hmm, fixed everything except a test coverage percentage check, not sure what to do about that one.

jameshilliard · 2024-07-18T18:33:00Z

Tests passing now.

.github/workflows/ci.yml

janiversen · 2024-07-18T18:37:13Z

pyproject.toml

@@ -240,7 +240,7 @@ exclude_also = [
    "if __name__ == .__main__.:",
    ]
 skip_covered = true
-fail_under = 90.0
+fail_under = 89.0


A not acceptable change, we want to have a higher % not lower !!

Hmm, since this code is going to be overhauled anyways does test coverage percentage matter all that much right now? Getting typing working better should hopefully make it easier to identify unused codepaths and do further cleanup so I'm not sure we should be actually be all that concerned about coverage at the moment for things that are being refactored like the transaction code. I'm suspecting that there's some unused code branches in the sync client transaction code.

Yes it matters, we do not merge a PR that lowers the %..

As a principle all new code should have 100% coverage, and this PR contains untested code

You do not need to “suspect”, use “pytest —cov” and you know.

Exactly refactoring code is a good reason to have near 100% coverage before starting, because it helps detect side effects.

You do not need to “suspect”, use “pytest —cov” and you know.

I mean, the code additions were all entirely to fix type check errors, I have no idea if some of those branches in the sync client transaction code can even be hit in practice.

Exactly refactoring code is a good reason to have near 100% coverage before starting, because it helps detect side effects.

Ideally but seems main problem is the type checker is flagging a bunch of issues(lack of checks for None for example) and I wouldn't know how to write tests to hit those conditions, the sync transaction code is super messy and hard to follow, at least with type checks we can start restricting the allowable types in various args/variables more easily.

Well maybe you have defined the wrong types, which at least seems to be a possibility.

If you change code, we need to verify why it (in your opinion) did not work earlier, and why it works now. I strongly believe that it is a sound and natural demand ! we do not do changes just because the typing you define causes problems.

That all being said, there might of and most likely are bugs in the code, which proper typing will bring to the surface, but I am sure you understand, why proof is needed..

If it's that hard to write tests, then maybe it is also hard to write the correct typing, and not just guessing what is correct !! meaning you could always reduce the PR to the part where you know how to write tests.

Adding types to code you do not understand, it quite a mountain to climb !! it is hard enough to add typing to code you understand. Just to give you an example, we had discussions during month how to add typing to mixin.py until someone suggested templates.

Well maybe you have defined the wrong types, which at least seems to be a possibility.

The tests also appear to use the wrong types in a number of places which is why I was trying to fix those in #2249.

If it's that hard to write tests, then maybe it is also hard to write the correct typing, and not just guessing what is correct !! meaning you could always reduce the PR to the part where you know how to write tests.

It's also hard to use the tests to validate typing issues when there's a lot of typing issues being flagged by mypy even on dev.

Adding types to code you do not understand, it quite a mountain to climb !! it is hard enough to add typing to code you understand. Just to give you an example, we had discussions during month how to add typing to mixin.py until someone suggested templates.

If we fix the typing errors in the tests it makes it easier to validate that the types here are correct in transaction.py doesn't it?

pyproject.toml

janiversen · 2024-07-18T18:40:39Z

And just for the fun of it, I run into lowering the % in #2239, but instead of lowering the bar, I will examine the cause and add tests.

janiversen

May I politely suggest, you do typing in this PR and not a lot of code changes !

The typing in this PR is OK, the code changes are not covered by tests, and a number of the changes seems wrong.

janiversen · 2024-07-19T06:55:28Z

pyproject.toml

@@ -240,7 +240,7 @@ exclude_also = [
    "if __name__ == .__main__.:",
    ]
 skip_covered = true
-fail_under = 90.0
+fail_under = 89.0


Yes it matters, we do not merge a PR that lowers the %..

As a principle all new code should have 100% coverage, and this PR contains untested code

janiversen · 2024-07-19T07:22:26Z

pyproject.toml

@@ -240,7 +240,7 @@ exclude_also = [
    "if __name__ == .__main__.:",
    ]
 skip_covered = true
-fail_under = 90.0
+fail_under = 89.0


You do not need to “suspect”, use “pytest —cov” and you know.

Exactly refactoring code is a good reason to have near 100% coverage before starting, because it helps detect side effects.

janiversen · 2024-07-19T09:09:46Z

pymodbus/transaction.py

-                isinstance(size, bytes)
-                and self.client.state == ModbusTransactionState.RETRYING
-            ):
+            if self.client.state == ModbusTransactionState.RETRYING:


This change do not look correct, we only want to enter the "if" if we received something.

This have nothing to do with typing, and it is not covered by tests.

I removed the isinstance(size, bytes) condition, which based on the return type of self._send(packet) is always an integer. This was flagged by the type checker.

But it still needs to get argumented and tested. I see quite a different behavior, because you return a fixed b"", which basically never should be returned in a retry situation.

I see quite a different behavior, because you return a fixed b"", which basically never should be returned in a retry situation.

I removed the return entirely and changed this check to verify returned size is not 0.

janiversen · 2024-07-19T09:10:23Z

pymodbus/transaction.py

                Log.debug(
                    "Changing transaction state from "
                    '"RETRYING" to "PROCESSING REPLY"'
                )
                self.client.state = ModbusTransactionState.PROCESSING_REPLY
-                return size, None
+                return b"", None


This is wrong, we only change to REPLY when we have a reply.

This have nothing to do with typing, and it is not covered by tests.

This is wrong, we only change to REPLY when we have a reply.

The issue here is that we can't return an integer, the calling code expects bytes to be returned.

The code as it is works or not ?

In any case a test case would show the current code to be faulty, and your new code to be correct.

The code as it is works or not ?

I guess? I mean the tests seem to pass either way so it's a bit hard to tell.

I went ahead and removed this return entirely which also seems to work.

janiversen · 2024-07-19T09:11:20Z

pymodbus/transaction.py

@@ -420,8 +423,10 @@ def _recv(self, expected_response_length, full) -> bytes:  # noqa: C901
                min_size = 4
            elif isinstance(self.client.framer, ModbusAsciiFramer):
                min_size = 5
-            else:
+            elif expected_response_length:


This do not seem right, however it is not covered by tests.

This was flagged by the type-checker, if expected_response_length is None we can't assign min_size to expected_response_length as we pass min_size to self.client.framer.recvPacket which expects an integer.

Seems you need more typing.

Apparently tweaking the style here a bit using original behavior got it passing the type checker.

janiversen · 2024-07-19T09:11:44Z

pymodbus/transaction.py

@@ -463,13 +468,17 @@ def _recv(self, expected_response_length, full) -> bytes:  # noqa: C901
                        expected_response_length -= min_size
                        total = expected_response_length + min_size
                else:
+                    if exception_length is None:


Not covered by test.

This was flagged by the type-checker, if exception_length is None we would try to subtract from exception_length in expected_response_length = exception_length - min_size which would not work.

What has that to do with that you have not added test cases, that show why the old code was broken and the new code works.

What has that to do with that you have not added test cases, that show why the old code was broken and the new code works.

Tweaked this a bit more to get it passing the type checker a slightly different way, I'm not sure how we would actually hit this codepath however so a bit hard to write tests.

janiversen · 2024-07-19T09:11:55Z

pymodbus/transaction.py

                    expected_response_length = exception_length - min_size
                    total = expected_response_length + min_size
            else:
                total = expected_response_length
        else:
            read_min = b""
            total = expected_response_length
+        if expected_response_length is None:


Not covered by test.

This was flagged by the type-checker, expected_response_length can't be None as we pass expected_response_length to self.client.framer.recvPacket which expects an integer.

Did you read my comment ?

None can be converted to a 0 without problems.

None can be converted to a 0 without problems.

Updated to no longer convert to 0.

jameshilliard · 2024-07-19T09:45:06Z

May I politely suggest, you do typing in this PR and not a lot of code changes !

All code changes made are directly due to issues being flagged by the typing additions.

janiversen · 2024-07-19T10:17:01Z

Well some of them are wrong.

janiversen · 2024-07-19T10:17:48Z

Please see my review comments, and when done request a new review.

janiversen · 2024-07-19T18:19:13Z

Adding typing is not always easy, I know @alexrudd2 have had some difficult times with a lot of the code.

BUT, when changing code that have been working for years, then it is important to show why the change is correct, that is most easily done with test cases, which we can run on the old code (dev) and the new code (this PR).

alexrudd2

I am commenting only on the use of from typing import Union.

Although I think the other typing changes are worthwhile (in particular the existing code does not perfectly handle None or ModbusException), when I last looked at them I was unable to find a clean solution.

alexrudd2 · 2024-07-19T18:56:15Z

pymodbus/client/base.py

 from dataclasses import dataclass
-from typing import Any, cast
+from typing import Any, Union, cast


Use from __future__ import annotations to match the rest of the codebase.

This allows using X | Y for instead of Union[X | Y] Union types

Use from __future__ import annotations to match the rest of the codebase.

pymodbus/pymodbus/client/base.py

Line 2 in 72e3399

from __future__ import annotations

That import already exists.

This allows using X | Y for instead of Union[X | Y] Union types

Not sure why exactly, but the normal style doesn't seem to work here.

??

Works for me with both pyright and mypy. Did you mismatch a bracket, perhaps?

Works for me with both pyright and mypy. Did you mismatch a bracket, perhaps?

Oh, the error is coming from pylint, not pyright or mypy:

$ pylint --recursive=y examples pymodbus test ************* Module pymodbus.client.base pymodbus/client/base.py:23:61: E1131: unsupported operand type(s) for | (unsupported-binary-operation) pymodbus/client/base.py:228:45: E1131: unsupported operand type(s) for | (unsupported-binary-operation)

Bah, appears to be a pylint bug. pylint-dev/pylint#7381.

Use # pylint: disable=unsupported-binary-operation, I guess.

No problem with adding them, but in this case you will need to add from __future__ import annotations for the first time.

Tried this, doesn't seem to help, looks to just be a limitation of python 3.9.

I agree with @alexrudd2 adding typing to tests, hav3 no real value and will most likely just complicate writing tests.

Please do not go down that route.

adding typing to tests, hav3 no real value and will most likely just complicate writing tests.

So I'm not really trying to add types to the tests so much as make it so that we don't get type errors when we typecheck the tests.

Please do NOT add typing to test, nor do not try to typecheck tests !

typecchecking tests is an unwanted feature.

Even if we were to consider it, you should NOT mix production code changes with test changes.

typecchecking tests is an unwanted feature.

I'm still confused about this, I mean if the tests have incorrect types then they are likely to cause issues.

Even if we were to consider it, you should NOT mix production code changes with test changes.

Yeah, going to split things out more.

janiversen · 2024-07-20T05:56:56Z

Please stop extending this PR, you still have not solved the basic problems in transaction.py

Adding test files to the mix, will only ensure that you will not get the PR reviewed !

jameshilliard · 2024-07-20T07:54:36Z

Please stop extending this PR, you still have not solved the basic problems in transaction.py

Yeah, I should probably split off those test fixes, it ended up being more fixes than I expected. Was trying to fix the mypy typing errors in tests calling transaction.py to make it easier to validate typing there. I found a few issues already with the tests not always using the correct types at least.

janiversen · 2024-07-20T08:09:41Z

the only reason to mix test files with production code, is if the production code changes needs changes in test !!!

That is an unbreakable rule, just like the CI rule.

However I can just as well tell you, your test changes are unwanted because the only complicate tests and does nothing to improve the production code.

jameshilliard · 2024-07-20T16:56:13Z

However I can just as well tell you, your test changes are unwanted because the only complicate tests and does nothing to improve the production code.

I'm confused, the test changes are fixing bugs in the tests flagged by mypy. If the mocking in the tests do not use the correct types then the tests are much less useful aren't they?

janiversen · 2024-07-20T20:13:01Z

We do not run mypy and a number of the other tools an test, specifically because there are a larger number of lines flagged, which actually are ok or even needed.

janiversen · 2024-07-20T20:14:29Z

And can we please agree "bugs" are when something do not want as we want, violating types is surely not in that category (and in many cases because we want it).

janiversen

There are code changes, which seems untested, and I believe the code changes might be incorrect.

Would be nice to have a test that shows why the old code was wrong, and the new code solves the problem.

janiversen · 2024-07-20T06:01:42Z

.github/workflows/ci.yml

@@ -93,7 +93,7 @@ jobs:
      - name: mypy
        if: matrix.run_lint == true
        run: |
-          mypy pymodbus
+          mypy pymodbus test/test_transaction.py test/transport/test_comm.py 


This is an unwanted change, that will not be accepted !

Why are you adding ci.yaml when you know that is not the way to make CI changes ?

I am confused, what are you trying to achieve, it is surely not to get this PR merged, please explain your objective ?

To be honest, these new additions makes me wonder if you want to help the project, or just waste time (sorry to be blunt).

Why are you adding ci.yaml when you know that is not the way to make CI changes ?

Was trying to use CI to validate some tests(there was a flaky test that was failing on windows). I removed this now.

janiversen · 2024-07-20T06:02:55Z

test/framers/test_old_framers.py

@@ -1,4 +1,6 @@
 """Test framers."""
+from __future__ import annotations


This is wrong, we only add future in files where it is needed.

janiversen · 2024-07-20T08:01:40Z

pymodbus/client/base.py

 from dataclasses import dataclass
-from typing import Any, cast
+from typing import Any, Union, cast


Please do NOT add typing to test, nor do not try to typecheck tests !

typecchecking tests is an unwanted feature.

Even if we were to consider it, you should NOT mix production code changes with test changes.

janiversen · 2024-07-20T20:16:55Z

pymodbus/transaction.py

        """Receive."""
        total = None
        if not full:
            exception_length = self._calculate_exception_length()
+            min_size = expected_response_length


This change makes no sense, you set the variable just to overwrite it, that is pure waste.

For some reason mypy wasn't liking the previous version here. Not sure what the best approach would be.

jameshilliard · 2024-07-20T23:41:18Z

We do not run mypy and a number of the other tools an test, specifically because there are a larger number of lines flagged, which actually are ok or even needed.

I'm aware there are currently a lot of mypy errors with the tests...but I see no indication that these aren't fixable by cleaning up the test code in various ways, for example this eliminates method-assign mypy errors in test/test_transaction.py while also eliminating some ugly hacks with how the mocking was being done.

And can we please agree "bugs" are when something do not want as we want, violating types is surely not in that category (and in many cases because we want it).

I mean, tests using the wrong types are kinda a problem IMO as it makes it more difficult to tell if the production code is correct.

alexrudd2 · 2024-07-21T05:43:23Z

And can we please agree "bugs" are when something do not want as we want, violating types is surely not in that category (and in many cases because we want it).

Here I must disagree. :) Although mypy has some false positives, making it happy fixes bugs and prevents new ones. I'd say 20% of the time type errors are actual logic errors, which is high enough to make it worthwhile.

janiversen · 2024-07-21T06:52:03Z

Here I must disagree. :) Although mypy has some false positives, making it happy fixes bugs and prevents new ones. I'd say 20% of the time type errors are actual logic errors, which is high enough to make it worthwhile.

Well if there are actual logic errors, then the code do not work as we want, and thus it is a bug.....problem is that mypy is very do not allow to use the full power of python, being a non typed language, e.g. assigning different types to a variable makes mypy mark it, but it's legal and often in test a nice way to test different parameter types,

jameshilliard · 2024-07-21T07:53:06Z

Well if there are actual logic errors, then the code do not work as we want, and thus it is a bug.....problem is that mypy is very do not allow to use the full power of python

I mean, I think there's usually ways to make tests play nice with mypy while making the test code cleaner like in #2251 and for cases where it's not possible can just add ignores.

being a non typed language

Python is both a strongly typed and a dynamically typed language, it's certainly not a "non typed language" whatever that means.

assigning different types to a variable makes mypy mark it, but it's legal and often in test a nice way to test different parameter types,

IMO not being sufficiently strict with types is a major issue that makes the transaction code hard to work with.

janiversen · 2024-07-21T08:07:20Z

I mean, I think there's usually ways to make tests play nice with mypy while making the test code cleaner like in #2251 and for cases where it's not possible can just add ignores.

I am sure there are.

Python is both a strongly typed and a dynamically typed language, it's certainly not a "non typed language" whatever that means.

It is a non typed language, because you can start using variables without giving them a type, and a variable freely be assigned all kind of types. Python allows typing, but the interpreter do not use that information, which is the definition of a non typed language.

IMO not being sufficiently strict with types is a major issue that makes the transaction code hard to work with.

Yes but it one of the strong features of python, if you want strict typing then start using e,g, C++

jameshilliard · 2024-07-21T08:37:17Z

It is a non typed language, because you can start using variables without giving them a type, and a variable freely be assigned all kind of types.

So...dynamically typed I think would be what you're referring to here. Where did the term "no typed language" come from? I'm not seeing much of anything in google about that term.

Python allows typing, but the interpreter do not use that information, which is the definition of a non typed language.

This is...not accurate at all, what do you think a TypeError is if the interpreter doesn't use typing information?

Python is a strongly typed language, strong typing means that variables do have a type and that the type matters when performing operations on a variable.

Yes but it one of the strong features of python

I think you're mixing up Python with JavaScript-like languages that are both weakly typed and dynamically typed. Python's type system is more lenient than C++ while still being strongly typed which I prefer to weakly typed languages like JavaScript.

if you want strict typing then start using e,g, C++

C++ is statically typed with fairly strong typing. Python is dynamically typed with similarly strong typing(I think in some ways even stronger than C++), one reason I like Python is due to it being dynamically and strongly typed. I try to stay away from JavaScript-like languages when possible in general primarily due to them using weak typing which generally results in more bugs.

janiversen · 2024-07-21T09:14:58Z

So...dynamically typed I think would be what you're referring to here. Where did the term "no typed language" come from? I'm not seeing much of anything in google about that term.

No dynamically typed (e,g, java) is different, as the name indicates a type is added dynamically, and the interpreter uses that to e.g. assign storage.

This is...not accurate at all, what do you think a TypeError is if the interpreter doesn't use typing information?

For the interpreter everything is stored in a object class, of course within that class there can be types.

I think you're mixing up Python with JavaScript-like languages that are both weakly typed and dynamically typed. Python's type system is more lenient than C++ while still being strongly typed which I prefer to weakly typed languages like JavaScript.

No I am not ! I have been around from before all these fancy languages, and spent quite a lot of time helping make parsers and interpreters.

Fact is, you can write a perfectly valid python program, without using a single type definition, and if you care to take a look at the python parser, it skips all type information, which is actually currently one of the bigger problems for making an optimizer in coython.

Of course the object class used by the runtime, do checks on assignment etc, but that is due to the internals of the object, not the interpreter,

C++ is statically typed with fairly strong typing. Python is dynamically typed with similarly strong typing(I think in some ways even stronger than C++), one reason I like Python is due to it being dynamically and strongly typed. I try to stay away from JavaScript-like languages when possible in general primarily due to them using weak typing which generally results in more bugs.

Your sentence is wrong. python allow typing, but do not use that information as it is purely optional.

Easy proof, make an array with 1.000.000 entries with a mixture of integers and floats, measured the actual memory footprint. Then type the array and use only integers, the active memory footprint is the same. A typed language would use the typing information to optimize the memory footprint.

But anyhow believe what you want to believe, I am too old to give free lessons in compiler techniques.

jameshilliard · 2024-07-21T10:23:30Z

No dynamically typed (e,g, java) is different, as the name indicates a type is added dynamically, and the interpreter uses that to e.g. assign storage.

Java is actually statically typed, JavaScript is dynamically typed at least based on the definitions that I'm familiar with:
"A statically-typed language is a language (such as Java, C, or C++) where variable types are known at compile time. In most of these languages, types must be expressly indicated by the programmer; in other cases (such as OCaml), type inference allows the programmer to not indicate their variable types."

For the interpreter everything is stored in a object class, of course within that class there can be types.

Everything may be an object but "there can be types" isn't very precise as "Every object has an identity, a type and a value.", I think this is somewhat important for making python's strong typing system work.

Fact is, you can write a perfectly valid python program, without using a single type definition, and if you care to take a look at the python parser, it skips all type information, which is actually currently one of the bigger problems for making an optimizer in coython.

That's true, "The Python runtime does not enforce function and variable type annotations. They can be used by third party tools such as type checkers, IDEs, linters, etc.". It is of course also true that it's harder to optimize dynamically typed languages like Python.

Your sentence is wrong. python allow typing, but do not use that information as it is purely optional.

The sort of typing I think you are referring to is just optional type annotations, so it is true that typing is optional in that sense. However "In a dynamically typed language, a variable is simply a value bound to a name; the value has a type -- like "integer" or "string" or "list" -- but the variable itself doesn't.".

Easy proof, make an array with 1.000.000 entries with a mixture of integers and floats, measured the actual memory footprint. Then type the array and use only integers, the active memory footprint is the same. A typed language would use the typing information to optimize the memory footprint.

That's true, "The Python runtime does not enforce function and variable type annotations.". "In a dynamically typed language, objects still have a type, but it is determined at runtime."

But anyhow believe what you want to believe, I am too old to give free lessons in compiler techniques.

I think for the most part you are just using different terminology than what I'm familiar with.

janiversen · 2024-07-21T10:54:45Z

Most people do not care about how the compiler works as long as it does the job, which is a pity because it often leads to programs with a bad performance.

If you solely look at typing from the outside, you really miss a lot, in order to make big scale programs.

As you correctly noted dynamically typed means a variable is assigned a type at runtime...but you missed the point everything in python is inherited from THE Object so the type of everything in python is Object, This is the reason why you can e,g, assign a string to a function, and thus totally change how it works.

janiversen · 2024-07-21T11:00:18Z

But as I said, stay in your beliefs!

Fact for this project, typing is accepted because a couple of contributors care about it. Every change that complicates the code, without adding other benefit than pleasing mypy is not accepted.

jameshilliard · 2024-07-21T21:28:29Z

Most people do not care about how the compiler works as long as it does the job, which is a pity because it often leads to programs with a bad performance.

I am heavily involved with python toolchain/package integration as part of the open source buildroot embedded Linux cross-compilation project and have worked on complex cross language toolchain integrations such as buildroot's python rust extension integration(used by libraries like python-cryptography which use pyo3 bindings). So I'm not at all unfamiliar with a lot of lower level details regarding the Python toolchain/interpreter. Python is not a particular easy language to cross compile either but buildroot has excellent support for python cross compilation partially due to a lot of work I've done on it(pep517 build system integrations for example).

you missed the point everything in python is inherited from THE Object so the type of everything in python is Object, This is the reason why you can e,g, assign a string to a function, and thus totally change how it works.

I don't think I'm disagreeing with that, that's just an implementation detail of python's dynamic typing system.

github-actions · 2024-09-05T02:26:52Z

This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.

github-actions · 2024-09-15T02:35:28Z

This PR was closed because it has been stalled for 10 days with no activity.

jameshilliard force-pushed the typing branch from a1eb85b to cd6a441 Compare July 18, 2024 17:55

janiversen reviewed Jul 18, 2024

View reviewed changes

jameshilliard force-pushed the typing branch 2 times, most recently from f246ab2 to 21700d8 Compare July 18, 2024 18:08

jameshilliard force-pushed the typing branch 4 times, most recently from 94b17d8 to 6b14160 Compare July 18, 2024 18:28

janiversen requested changes Jul 18, 2024

View reviewed changes

pyproject.toml Outdated Show resolved Hide resolved

jameshilliard force-pushed the typing branch 2 times, most recently from 1a9160e to 2cb3354 Compare July 18, 2024 19:53

janiversen requested changes Jul 19, 2024

View reviewed changes

janiversen marked this pull request as draft July 19, 2024 10:18

jameshilliard force-pushed the typing branch 8 times, most recently from 05b3826 to 3ddd6f6 Compare July 19, 2024 18:00

alexrudd2 requested changes Jul 19, 2024

View reviewed changes

janiversen mentioned this pull request Jul 20, 2024

Block draft PRs to use CPU minutes #2245

Merged

jameshilliard force-pushed the typing branch 3 times, most recently from 3554a1c to e4625fa Compare July 20, 2024 17:08

Add more typing to transaction and base client

b3bd579

jameshilliard force-pushed the typing branch from e4625fa to b3bd579 Compare July 20, 2024 17:09

jameshilliard marked this pull request as ready for review July 20, 2024 17:14

janiversen requested changes Jul 20, 2024

View reviewed changes

janiversen marked this pull request as draft July 20, 2024 20:19

github-actions bot added the no-pr-activity label Sep 5, 2024

github-actions bot closed this Sep 15, 2024

		@@ -1,4 +1,6 @@
		"""Test framers."""
		from __future__ import annotations

Add more typing to transaction and base client #2238

Add more typing to transaction and base client #2238

Conversation

jameshilliard commented Jul 18, 2024

janiversen left a comment

Choose a reason for hiding this comment

jameshilliard commented Jul 18, 2024

jameshilliard commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janiversen commented Jul 18, 2024

janiversen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameshilliard Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameshilliard commented Jul 19, 2024

janiversen commented Jul 19, 2024

janiversen commented Jul 19, 2024

janiversen commented Jul 19, 2024

alexrudd2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janiversen commented Jul 20, 2024 • edited Loading

jameshilliard commented Jul 20, 2024

janiversen commented Jul 20, 2024

jameshilliard commented Jul 20, 2024

janiversen commented Jul 20, 2024

janiversen commented Jul 20, 2024

janiversen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameshilliard commented Jul 20, 2024

alexrudd2 commented Jul 21, 2024

janiversen commented Jul 21, 2024

jameshilliard commented Jul 21, 2024

janiversen commented Jul 21, 2024

jameshilliard commented Jul 21, 2024

janiversen commented Jul 21, 2024

jameshilliard Jul 19, 2024 •

edited

Loading

janiversen commented Jul 20, 2024 •

edited

Loading